Fix panic with plugins and build commands
authorAlex Crichton <alex@alexcrichton.com>
Mon, 17 Nov 2014 17:35:53 +0000 (09:35 -0800)
committerAlex Crichton <alex@alexcrichton.com>
Mon, 17 Nov 2014 17:35:53 +0000 (09:35 -0800)
This fixes a bug in cargo where target executables and libraries would be linked
to plugin native dependencies (not wanted!).

Closes #885

src/cargo/core/package.rs
src/cargo/ops/cargo_rustc/context.rs
src/cargo/ops/cargo_rustc/mod.rs
tests/test_cargo_compile_custom_build.rs

index fa84b9c3bf88a1340e16a90f51829822e61c9182..cdb57c6bf3d5a6e7b4d044cc53cc7b7dfbc404ee 100644 (file)
@@ -110,6 +110,10 @@ impl Package {
     pub fn get_absolute_target_dir(&self) -> Path {
         self.get_root().join(self.get_target_dir())
     }
+
+    pub fn has_custom_build(&self) -> bool {
+        self.get_targets().iter().any(|t| t.get_profile().is_custom_build())
+    }
 }
 
 impl Show for Package {
index 251aa64b9882d87208876a42020751dedde34af2..1e905facf1a1d0d2e496850c1f55fd8681df1fc6 100644 (file)
@@ -326,4 +326,21 @@ impl PlatformRequirement {
             _ => PlatformPluginAndTarget,
         }
     }
+
+    pub fn includes(self, kind: Kind) -> bool {
+        match (self, kind) {
+            (PlatformPluginAndTarget, _) |
+            (PlatformTarget, KindTarget) |
+            (PlatformPlugin, KindHost) => true,
+            _ => false,
+        }
+    }
+
+    pub fn each_kind(self, f: |Kind|) {
+        match self {
+            PlatformTarget => f(KindTarget),
+            PlatformPlugin => f(KindHost),
+            PlatformPluginAndTarget => { f(KindTarget); f(KindHost); }
+        }
+    }
 }
index bcea5a073c55159435d5e0348d221f21ca5e244e..56c7154c5ed0a441a54e0c061db244f32dc6adfd 100644 (file)
@@ -390,30 +390,38 @@ fn rustc(package: &Package, target: &Target,
         let build_state = cx.build_state.clone();
         let mut native_lib_deps = HashSet::new();
         let current_id = package.get_package_id().clone();
-        let has_custom_build = package.get_targets().iter().any(|t| {
-            t.get_profile().is_custom_build()
-        });
 
-        if has_custom_build && !target.get_profile().is_custom_build() {
+        if package.has_custom_build() && !target.get_profile().is_custom_build() {
             native_lib_deps.insert(current_id.clone());
         }
         // Visit dependencies transitively to figure out what our native
         // dependencies are (for -L and -l flags).
-        for &(pkg, _) in cx.dep_targets(package, target).iter() {
-            each_dep(pkg, cx, |dep| {
-                let has_custom_build = dep.get_targets().iter().any(|t| {
-                    t.get_profile().is_custom_build()
-                });
-                if has_custom_build {
-                    native_lib_deps.insert(dep.get_package_id().clone());
+        //
+        // Be sure to only look at targets of the same Kind, however, as we
+        // don't want to include native libs of plugins for targets for example.
+        fn visit<'a>(cx: &'a Context, pkg: &'a Package, target: &Target,
+                     kind: Kind,
+                     visiting: &mut HashSet<&'a PackageId>,
+                     libs: &mut HashSet<PackageId>) {
+            for &(pkg, target) in cx.dep_targets(pkg, target).iter() {
+                let req = cx.get_requirement(pkg, target);
+                if !req.includes(kind) { continue }
+                if !visiting.insert(pkg.get_package_id()) { continue }
+
+                if pkg.has_custom_build() {
+                    libs.insert(pkg.get_package_id().clone());
                 }
-            });
+                visit(cx, pkg, target, kind, visiting, libs);
+                visiting.remove(&pkg.get_package_id());
+            }
         }
+        visit(cx, package, target, kind,
+              &mut HashSet::new(), &mut native_lib_deps);
         let mut native_lib_deps = native_lib_deps.into_iter().collect::<Vec<_>>();
         native_lib_deps.sort();
 
-        // If we are a binary and the package also contains a library, then we don't
-        // pass the `-l` flags.
+        // If we are a binary and the package also contains a library, then we
+        // don't pass the `-l` flags.
         let pass_l_flag = target.is_lib() || !package.get_targets().iter().any(|t| {
             t.is_lib()
         });
@@ -503,10 +511,7 @@ fn rustdoc(package: &Package, target: &Target,
 
     let mut rustdoc = try!(build_deps_args(rustdoc, target, package, cx, kind));
 
-    let has_build_cmd = package.get_targets().iter().any(|t| {
-        t.get_profile().is_custom_build()
-    });
-    rustdoc = rustdoc.env("OUT_DIR", if has_build_cmd {
+    rustdoc = rustdoc.env("OUT_DIR", if package.has_custom_build() {
         Some(cx.layout(package, kind).build_out(package))
     } else {
         None
@@ -663,10 +668,7 @@ fn build_deps_args(mut cmd: ProcessBuilder, target: &Target, package: &Package,
     cmd = cmd.arg("-L").arg(layout.root());
     cmd = cmd.arg("-L").arg(layout.deps());
 
-    let has_build_cmd = package.get_targets().iter().any(|t| {
-        t.get_profile().is_custom_build()
-    });
-    cmd = cmd.env("OUT_DIR", if has_build_cmd {
+    cmd = cmd.env("OUT_DIR", if package.has_custom_build() {
         Some(layout.build_out(package))
     } else {
         None
index 5fd4bcfeda2bbf0d175804ef801877f5be696532..f373c7440b64de5974441335dc34b728e52e46df 100644 (file)
@@ -897,3 +897,45 @@ test!(shared_dep_with_a_build_script {
     assert_that(p.cargo_process("build"),
                 execs().with_status(0));
 })
+
+test!(transitive_dep_host {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.5.0"
+            authors = []
+            build = "build.rs"
+
+            [build-dependencies.b]
+            path = "b"
+        "#)
+        .file("src/lib.rs", "")
+        .file("build.rs", "fn main() {}")
+        .file("a/Cargo.toml", r#"
+            [package]
+            name = "a"
+            version = "0.5.0"
+            authors = []
+            links = "foo"
+            build = "build.rs"
+        "#)
+        .file("a/build.rs", "fn main() {}")
+        .file("a/src/lib.rs", "")
+        .file("b/Cargo.toml", r#"
+            [package]
+            name = "b"
+            version = "0.5.0"
+            authors = []
+
+            [lib]
+            name = "b"
+            plugin = true
+
+            [dependencies.a]
+            path = "../a"
+        "#)
+        .file("b/src/lib.rs", "");
+    assert_that(p.cargo_process("build"),
+                execs().with_status(0));
+})